API review

Proposer: Patrick

Present at review:

  • Tully
  • Radu
  • Josh
  • Kurt

Overview

The old channel-based sensor_msgs/PointCloud was found to be very awkward to use. It only supports float32 channels, forcing dangerous casting for other kinds of data. It also does not map well to how developers represent point cloud data structures, requiring a lot of data shuffling to put them in message format.

The new point cloud message represents point data as a binary blob of point structures. Metadata describes the names and types of the point fields, as well as their binary layout.

sensor_msgs/PointCloud2:

#This message holds a collection of nD points, as a binary blob.
Header header

# 2D structure of the point cloud. If the cloud is unordered,
# height is 1 and width is the length of the point cloud.
uint32 height
uint32 width

# Describes the channels and their layout in the binary data blob.
PointField[] fields

bool    is_bigendian # Is this data bigendian?
uint32  point_step   # Length of a point in bytes
uint32  row_step     # Length of a row in bytes
uint8[] data         # Actual point data, size is (row_step*height)

bool is_dense        # True if there are no invalid points

sensor_msgs/PointField:

#This message holds the description of one point entry in the PointCloud2 message format.
uint8 INT8    = 1
uint8 UINT8   = 2
uint8 INT16   = 3
uint8 UINT16  = 4
uint8 INT32   = 5
uint8 UINT32  = 6
uint8 FLOAT32 = 7
uint8 FLOAT64 = 8

string name     # Name of the field. Should follow standard conventions.
uint32 offset   # Byte offset from the start of the point
uint8  datatype # Enum for the scalar data type
uint32 size     # Number of elements

NOTE: The size field is a new addition in trunk. Previously it was very awkward to encode large array fields like float pfhDescriptor[81].

Wrapping user point types in C++

One of the motivating desires behind PointCloud2 is to allow the serialized data format to be the same as the in-memory format used by developers for collections of points. This makes the message more intuitive (despite being a binary blob) and allows (de)serialization of the point data to be a simple memcpy.

point_cloud_redesign contains proof-of-concept Publisher and Subscriber classes that translate between user-declared point structs and the PointCloud2 message. Somewhat like image_transport, they allow the user to send/receive their data in the most convenient format without worrying how it is actually represented over the wire. We don't need to debate the C++ API in this meeting, but I think it helps demonstrate the ease-of-use and efficiency arguments for the new message.

Registering a point struct with the system. This demonstrates using sub-structs and arrays:

   1 struct Position
   2 {
   3   float x;
   4   float y;
   5   float z;
   6 };
   7 
   8 struct MyPoint
   9 {
  10   Position pos;
  11   uint32_t w;
  12   float normal[3];
  13 };
  14 
  15 // Registration macro
  16 // The format is (type, field accessor, name)
  17 POINT_CLOUD_REGISTER_POINT_STRUCT(
  18   MyPoint,
  19   (float,    pos.x,   x)
  20   (float,    pos.y,   y)
  21   (float,    pos.z,   z)
  22   (uint32_t, w,       w)
  23   (float[3], normal, normal));

Publishing a point cloud:

   1 // Point cloud class templated on user point type,
   2 // intended for easy use in code.
   3 MyPoint pt;
   4 point_cloud::PointCloud<MyPoint> pt_cloud;
   5 pt_cloud.points.push_back(pt);
   6 pt_cloud.width = pt_cloud.height = 1;
   7 
   8 // Point cloud publisher understands how to translate the
   9 // user point type into a PointCloud2 message.
  10 point_cloud::Publisher<MyPoint> pub;
  11 pub.advertise(node_handle, "cloud_topic", 1);
  12 pub.publish(pt_cloud);

Subscribing to a point cloud:

   1 void cloudCallback(const point_cloud::PointCloud<MyPoint>::ConstPtr& msg)
   2 {
   3   printf("Cloud: width = %u, height = %u\n", msg->width, msg->height);
   4   BOOST_FOREACH(const Point& pt, msg->points) {
   5     printf("\tpos = (%f, %f, %f), w = %u, normal = (%f, %f, %f)\n",
   6            pt.pos.x, pt.pos.y, pt.pos.z, pt.w,
   7            pt.normal[0], pt.normal[1], pt.normal[2]);
   8   }
   9 }
  10 
  11 point_cloud::Subscriber<MyPoint> sub;
  12 sub.subscribe(node_handle, "cloud_topic", 1, cloudCallback);

Question / concerns / comments

Josh

  • size in PointField also lets you encode "xyz" as a single element... I assume we want to discourage that?

  • Is size in bytes? The comment seems to imply it's actually a count... if so I think count is a better name.

  • The comment for name says "should follow standard conventions". Can we put those conventions in the comments?

Meeting agenda

  • Optimizations
    • Publisher is already one memcpy
    • Nodelets?
    • Intra-process passing through shared_ptr to user data without PointCloud2 transformations (not done)

      • High priority for efficient nodelets
      • Under the hood, publish/subscribe the templated point cloud type. roscpp detects if the types are the same, will pass through the shared_ptr if they are.
      • Need to specialize Serializer on templated PointCloud.

    • Subscriber-side memcpy optimizations (not done)
      • Coalesce memcpy's for fields within the point struct
      • If Publisher/Subscriber agree exactly on format, do one memcpy
  • point_cloud_transport
    • Do we even need it? Just specialize ROS Publisher/Subscriber to do the right thing?
    • Holding up release of PCL. Need at least the PointCloud message filter.

Conclusion

Messages (Tully):

  • size -> count

PCL (Radu):

  • Move point_cloud_redesign magic into PCL as internal implementation details.

Point cloud transport (Patrick):

  • Evaluate whether can implement by customizing roscpp classes.
  • (Josh) Send me some sample code customizing Serializer with stateful create() function.
  • Plan to release immediately after ROS 1.1.

Package status change mark change manifest)

  • /!\ Action items that need to be taken.

  • {X} Major issues that need to be resolved


Wiki: sensor_msgs/Reviews/2010-03-01 PointCloud2_API_Review (last edited 2010-03-01 22:57:08 by PatrickMihelich)